Skip to content

Comments

WA-NEW-011: Remove use_dis_max from query_string queries (ES7 compatibility)#634

Open
kitcommerce wants to merge 1 commit intonextfrom
wa-new-011-es7-query-string
Open

WA-NEW-011: Remove use_dis_max from query_string queries (ES7 compatibility)#634
kitcommerce wants to merge 1 commit intonextfrom
wa-new-011-es7-query-string

Conversation

@kitcommerce
Copy link

Summary

Remove the deprecated use_dis_max option from query_string queries in ProductSearch and HelpSearch. This option was removed in Elasticsearch 7 and causes a 400 parsing_exception.

Closes #628

Client impact

None expected. use_dis_max was already the default behavior in ES5/6; removing it explicitly is a no-op for query semantics. The tie_breaker parameter (which replaces the dis_max behavior) is retained.

Verify

bundle exec ruby -Icore/test core/test/queries/workarea/search/product_search_test.rb

@kitcommerce
Copy link
Author

Dispatcher Build Gate Summary (local)

  • rubocop (diff-only): PASS (0 offenses)
  • brakeman: skipped (per repo build gate config; disabled)
  • tests (affected engines): PASS
    • core: PASS
    • admin: PASS
    • storefront: PASS

Note: test output still prints BSON Symbol deprecation warning (expected to be addressed by WA-NEW-010 / PR #635).

@kitcommerce kitcommerce added review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress review:security-done Review complete and removed review:security-pending Review in progress labels Feb 22, 2026
@kitcommerce
Copy link
Author

Security Review

Verdict: APPROVE (low risk)

Summary

This PR removes use_dis_max: true from ES query_string queries for ES7 compatibility. No new data flows or input handling changes introduced.

Notes

  • The primary security-sensitive surface remains the existing use of query_string (Lucene syntax). This PR does not materially worsen that risk, but keep in mind potential expensive/wildcard query abuse if sanitization/limits are insufficient.
  • Brakeman is skipped in this repo’s build gate; if feasible, consider running it in CI for PRs touching query construction.

@kitcommerce kitcommerce added review:architecture-done Review complete and removed review:architecture-pending Review in progress labels Feb 22, 2026
@kitcommerce
Copy link
Author

Architecture Review

Verdict: APPROVE (high confidence)

Summary

Minimal, correct ES7 compatibility change: query_string.use_dis_max was removed in ES 7.x, so removing it avoids runtime query parsing errors without altering the rest of the query DSL.

Notes

  • Behavioral impact should be effectively none: the PR previously forced use_dis_max: true and that was historically the default for query_string.
  • tie_breaker remains in place; no configuration/extension surface changed.

Optional test ideas

  • Assert generated DSL for ProductSearch/HelpSearch no longer includes use_dis_max.
  • If feasible, run an ES7-backed integration/spec to catch any remaining deprecated query_string options.

@kitcommerce kitcommerce added review:simplicity-done Review complete and removed review:simplicity-pending Review in progress labels Feb 22, 2026
@kitcommerce
Copy link
Author

Simplicity Review

Verdict: APPROVE (high confidence)

Summary

Smallest, clearest ES7 compatibility fix: remove unsupported use_dis_max from query_string queries in HelpSearch/ProductSearch. No additional behavioral complexity introduced.

Note (optional)

  • Consider a quick repo-wide grep for use_dis_max to ensure there are no remaining ES7-breaking occurrences.
  • Optional: a query-building spec asserting deprecated query_string params are absent under ES7.

@kitcommerce kitcommerce added review:rails-conventions-done Rails conventions review complete and removed review:rails-conventions-pending Rails conventions review in progress labels Feb 22, 2026
@kitcommerce
Copy link
Author

Rails / Workarea Conventions Review

Verdict: APPROVE

Summary

Conventions-friendly, minimal change confined to query building: removes ES7-unsupported use_dis_max from query_string clauses in HelpSearch/ProductSearch.

Notes / watch-outs (non-blocking)

  • Possible subtle relevance/ranking shifts since use_dis_max previously forced dis_max semantics for multi-field query_string.
  • tie_breaker remains in ProductSearch; worth confirming ES7 still accepts/uses it for query_string (or that it is safely ignored).
  • If this code must support ES < 7 in some configs, consider version-conditional inclusion; otherwise this is fine.

Suggested follow-ups

  • Spec asserting ES7 query JSON does not include deprecated use_dis_max.
  • Light relevance spot-check on a few representative multi-field queries to catch ranking regressions.

@kitcommerce kitcommerce added the merge:ready All conditions met, eligible for merge label Feb 22, 2026
@kitcommerce
Copy link
Author

✅ Wave 1 Passed — Merge Ready

Wave 1 reviewers all returned APPROVE:

  • architecture: ✅
  • simplicity: ✅
  • security: ✅
  • rails-conventions: ✅

Build gate: gate:build-passed

Labeling this PR merge:ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate:build-passed Build gate passed merge:ready All conditions met, eligible for merge review:architecture-done Review complete review:rails-conventions-done Rails conventions review complete review:security-done Review complete review:simplicity-done Review complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant